Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: more spacing in edit preferences form #9097

Merged
merged 10 commits into from
Oct 19, 2023
Merged

Conversation

Valimp
Copy link
Collaborator

@Valimp Valimp commented Oct 2, 2023

Fixes #7492

What

Add 1rem of margin bottom to close button and "Classify products according to your preferences"

Screenshot

spacing

Related issue(s) and discussion

@raphael0202

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2023

Codecov Report

Merging #9097 (d154f21) into main (d1481ab) will increase coverage by 0.10%.
Report is 12 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #9097      +/-   ##
==========================================
+ Coverage   47.85%   47.95%   +0.10%     
==========================================
  Files          64       64              
  Lines       19965    20013      +48     
  Branches     4833     4850      +17     
==========================================
+ Hits         9554     9597      +43     
- Misses       9163     9168       +5     
  Partials     1248     1248              

see 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stephanegigandet
Copy link
Contributor

Hi @Valimp , it's an improvement, but it doesn't fix the issue #7492 that you reference in the PR.
The issue is that the panel takes too much horizontal space for margins and padding on mobile:

image

One solution could be to use a modal instead:

https://get.foundation/sites/docs-v5/components/reveal.html

@raphael0202
Copy link
Contributor

As Stephane said it's an improvement but unrelated to the issue. Can you take care of fixing the original issue too in this PR?

And by the way margin-bottom is also missing below the "Close" button:

Capture d’écran 2023-10-03 à 10 32 17

@@ -222,10 +223,12 @@ function display_user_product_preferences(target_selected, target_selection_form
checked = ' checked';
}

attribute_group_html += "<div class='attribute_item'>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you add it below instead of adding a new attribute_group_html += ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's done

@raphael0202 raphael0202 self-requested a review October 19, 2023 15:37
@sonarcloud
Copy link

sonarcloud bot commented Oct 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@raphael0202 raphael0202 enabled auto-merge (squash) October 19, 2023 15:41
@raphael0202 raphael0202 merged commit dd02264 into main Oct 19, 2023
13 checks passed
@raphael0202 raphael0202 deleted the fix_#7492_search_settings branch October 19, 2023 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NEW DESIGN - Personal search settings are too narrow in mobile mode
4 participants